Clear omitted bindData fields when nullMissing is enabled - #15950
Clear omitted bindData fields when nullMissing is enabled#15950jamesfredley wants to merge 5 commits into
Conversation
When nullMissing is true and an include allowlist is provided, omitted allowlisted properties are set to null. Default remains leave-unchanged. Assisted-by: Sisyphus:xai/grok-4.5 [gpt-coding]
There was a problem hiding this comment.
Pull request overview
Adds opt-in “null missing” semantics to bindData so that, when nullMissing: true is provided alongside an explicit include allowlist, included properties omitted from the binding source are actively cleared (null) rather than leaving stale values on the target object. This extends Grails’ web data binding behavior to better support typical “edit/update” form semantics without enabling the behavior by default.
Changes:
- Introduces a
nullMissingoption plumbed throughDataBinder→DataBindingUtils, and applies clearing only when an explicitincludelist is provided. - Implements missing-field clearing logic in
DataBindingUtils(including nested indexed collection paths and map-indexed paths). - Adds test coverage and updates documentation + upgrading notes to describe the new opt-in behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| grails-web-databinding/src/main/groovy/grails/web/databinding/DataBindingUtils.java | Adds nullMissing overloads and implements missing included-property clearing logic after binding. |
| grails-web-databinding/src/main/groovy/grails/web/databinding/DataBinder.groovy | Wires nullMissing: true from the bindData options map into the binding call. |
| grails-test-suite-web/src/test/groovy/org/grails/web/servlet/BindDataMethodTests.groovy | Adds new controller-backed specs covering nullMissing clearing, excludes, nested indexed paths, map paths, and bindable whitelist interactions. |
| grails-doc/src/en/ref/Controllers/bindData.adoc | Documents nullMissing usage and constraints (opt-in; requires include). |
| grails-doc/src/en/guide/upgrading/upgrading80x.adoc | Notes the behavior change for Grails 8.x upgrades (opt-in; only with include). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The observation regarding |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## fix/binddata-mass-assignment #15950 +/- ##
======================================================================
- Coverage 51.4072% 50.8298% -0.5774%
+ Complexity 17830 17675 -155
======================================================================
Files 2039 2041 +2
Lines 96004 96347 +343
Branches 16692 16827 +135
======================================================================
- Hits 49353 48973 -380
- Misses 39296 40034 +738
+ Partials 7355 7340 -15
🚀 New features to boost your workflow:
|
Preserve the strict binding allowlist while adding opt-in stale-data clearing, including indexed paths and explicit include handling. Assisted-by: opencode:gpt-5.6-sol
|
Final update pushed in The branch now incorporates #15947 as its deny-by-default security base and keeps Verification passed for |
Assisted-by: opencode:gpt-5.6-sol
…le-data Assisted-by: opencode:gpt-5.6-sol
|
Thanks for splitting this out — since #15950 is branched directly off #15947, its diff currently duplicates all of #15947's changes on top of the Would you be open to one of:
Whichever is easier on your end — just flagging so we don't duplicate review effort. |
|
PR updated to target fix/binddata-mass-assignment |
✅ All tests passed ✅🏷️ Commit: 62d4aad Learn more about TestLens at testlens.app. |
Description
Adds opt-in stale-data clearing to
bindData, stacked on the binding base from #15947.When
nullMissing: trueis supplied together with an explicitincludelist, included properties omitted from the source are set tonull. Existing behavior is unchanged when the option is absent, false, or no explicit include was supplied.Final contract
bindData(target, source, [include: [...], nullMissing: true])bindable: false, and framework-managed properties remain protectednullMissing; it cannot broaden normal request bindingnullMissing = falseBinding base (from #15947)
This branch merges #15947, which keeps data binding permissive by default and adds an opt-in deny-by-default secure mode (
grails.databinding.legacyBindableDefault=false). The merge preserves #15947's exact/nested-prefix binding predicate and itsbindable: falseenforcement (always honored, including the nestedMap-constructor fallback). Generated entries such asfoo_*orfoo.*cannot authorize a sibling property such asfoo_admin.Indexed includes and excludes are normalized only for
nullMissingpath matching. Explicit includes do not bypass recursivebindable: false, framework-property, or exclusion checks.Example
Verification
:grails-web-databinding:test:grails-test-suite-web:test:grails-test-suite-persistence:testBindDataMethodTestscoverage for public APIs, explicit and generated allowlists, null-include safety,bindable: false, framework properties, nested/indexed collections and maps, filters, excludes, malformed indexes, and wildcard sibling protectionHoldersconfig to avoid cross-spec leakageRelated
Contributor Checklist
bindDatareference and Grails 8 upgrade guide.ai-generated-starting-point.Assisted-by: Sisyphus:gpt-5.6-sol [gpt-coding]